Replace custom protobuf handlers with librespot-protocol #714
Conversation
SO9010
left a comment
SO9010
left a comment
There was a problem hiding this comment.
Looks great to me, I need @jacksongoode to look over this as I have not been active for a while... Thanks for the help!
jacksongoode
left a comment
jacksongoode
left a comment
There was a problem hiding this comment.
Generally this looks fine, does this mean we can remove a lot of the native code from protocol?
It means that we can remove the entire folder |
|
I would say go ahead, it makes sense to if all functionality appears to work. |
|
I've gone ahead and done this. All appears to work. Given that this is a huge chunk of removed code, I'd like to see how it might change the package size. |
Boy, do I have meager news for you 😅 Jokes aside, the size won't change that much given that all the protocols removed are still used, only from a different package now. As far as I can see in the CI builds, the size change is in margin of error, effectively 0. |
|
Let's do it, I think the convergence with librespot is likely the best path towards sustainability anyways. |
Replace custom protobuf handlers with librespot-protocol (jpochyla#714)
3 participants
In #693 we have introduced
librespot-protocoland its direct dependencyprotobuftopsst-core. The code now uses two different protobuf libraries. This PR aims to reduce it back to one.I have been unsuccessful at porting new protobuf specs to
quick-protobuf. I can't get the bash scripts inpsst-protocolrunning, nor could I reproduce the rust code generation for the already existing protobufs. I tried tackling the problem from the other side instead: Use more of the librespot protobuf code.